conditional fields of constexpr literal structs #989
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
youknowone
changed the title
youknowone
force-pushed
the
constexpr-conditional-field
branch
from
b890eef
to
ddd17f7
Compare
emilio
approved these changes
Oct 27, 2024
| // TODO: Some C++ versions (c++20?) now support designated | ||
| // initializers, consider generating them. | ||
| write!(out, "/* .{} = */ ", ordered_key); | ||
| self.write_literal(out, &lit.value); | ||
| if i + 1 != ordered_fields.len() { | ||
| write!(out, ", "); |
There was a problem hiding this comment.
This adds trailing white-space, please remove the space if is_constexpr?
| @@ -911,33 +911,58 @@ impl LanguageBackend for CLikeLanguageBackend<'_> { | |||
| fields, | |||
| path, | |||
| } => { | |||
| let allow_constexpr = self.config.constant.allow_constexpr && l.can_be_constexpr(); | |||
| let is_constexpr = self.config.language == Language::Cxx | |||
| && (self.config.constant.allow_static_const || allow_constexpr); | |||
| if self.config.language == Language::C { | |||
| write!(out, "({})", export_name); | |||
| } else { | |||
| write!(out, "{}", export_name); | |||
| } | |||
|
|
|||
| write!(out, "{{ "); | |||
There was a problem hiding this comment.
Same, this space needs to be removed.
|
Sorry for the lag getting to this btw :( |
youknowone
force-pushed
the
constexpr-conditional-field
branch
from
ddd17f7
to
df9f46e
Compare
|
@emilio Thank you for the review! I updated the both PRs |
emilio
force-pushed
the
constexpr-conditional-field
branch
from
df9f46e
to
62c856f
Compare
|
Could you rebase this please? Thanks! |
youknowone
force-pushed
the
constexpr-conditional-field
branch
from
62c856f
to
cea2ef2
Compare
|
rebased |
|
@emilio Could you check it again? |
youknowone
force-pushed
the
constexpr-conditional-field
branch
from
cea2ef2
to
2ed778c
Compare
|
I updated my local patch to this branch. It worked well for the project. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Partially fix #955 about constexpr
#988 is included in this PR
Because constexpr and defines require very different approach about this problem, I created another PR for constexpr fix while keeping the other one for define fix.